correctness: Should this be looking at VersionIdMarker in addition to KeyMarker?

Im not 100% sure about this.

Each object has a version ID, whether or not S3 Versioning is enabled. If S3 Versioning is not enabled, Amazon S3 sets the value of the version ID to null. If you enable S3 Versioning, Amazon S3 assigns a version ID value for the object. This value distinguishes that object from other versions of the same key.

I can add that field to the validation as a "just in case"

func (p *ListObjectVersionsPaginator) HasMorePages() bool {
	return p.firstPage || (p.keyMarker != nil && len(*p.keyMarker) != 0) && (p.versionIdMarker != nil && len(*p.versionIdMarker) != 0)
}

I would take a look at one of the SDKs that already has these paginators (e.g. Java) to understand the correct behavior.

Here is where this is determined in the Java SDK. It only looks at isTruncated to determine if there are more pages.
I think it makes sense and is more elegant.

correctness: This doesn't account for VersionIdMarker being duplicate, should it?

From the S3 docs:

Only Amazon S3 generates version IDs, and they cannot be edited. Version IDs are Unicode, UTF-8 encoded, URL-ready, opaque strings that are no more than 1,024 bytes long. The following is an example:

3sL4kqtJlcpXroDTDmJ+rmSpXd3dIbrHY+MTRCxf3vjVBH40Nr8X8gdRQBpUMLUo

Since they are unique identifiers, and cannot be set by the user, the chance of collision within a specific bucket are tiny.
What are your thoughts?

I would think it's more about any one parameter being a duplicate rather than collision being the concern. I'm not 100% sure though, maybe another thing to look to other SDKs w.r.t behavior.

fix: I think this may have been discussed offline at one point but we should be able to just store and use the IsTruncated flag on the output which is what Java does.

e.g.

return p.firstPage || p.isTruncated

fix: Same thing, store and use the IsTruncated flag and predicate off that

fix: This shouldn't be necessary. The paginator takes an interface for the client type:

type ListObjectVersionsAPIClient interface {
	ListObjectVersions(context.Context, *ListObjectVersionsInput, ...func(*Options)) (*ListObjectVersionsOutput, error)
}

You can just implement a test/mock version of this interface which removes the need for middleware or constructing a real client and allows you to directly control the outputs and observe/record the inputs to make assertions.

e.g.

// this is just an example
type mockListObjectVersionsClient struct {
    responses []*ListObjectVersionsOutput
    inputs []*ListObjectVersionsOutput
    requestCount int
}

func (c *mockListObjectVersionsClient) ListObjectVersions(ctx context.Context, input *ListObjectVersionsInput, options ...func(*Options)) (*ListObjectVersionsOutput, error) {
    idx := c.requestCount + 1
    if idx > len(c.resopnses) {   // error }
    c.requestCount = idx     
    // record the input observed
    c.inputs = append(c.inputs, input)
    return c.responses[idx], nil
}